🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20734

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 12 Awaiting Approval

As of commit 4c18238 with merge base a6d812a ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

The committers listed above are authorized under a signed CLA.

• ✅ login: SakshamKapoor2911 / name: SakshamKapoor2911 (e7636f5)

❌ 🤖 pytorchbot command failed:

Got EOF while in a quoted string```
Try `@pytorchbot --help` for more info.

@pytorchbot label "release notes: xnnpack"

The following ciflow label(s) have been added but CI has not been triggered yet because the workflows are awaiting approval:

• ciflow/trunk

Once a maintainer approves the workflows (scroll to the bottom of the PR page), the corresponding CI jobs will be triggered automatically. Please ping one of the reviewers if you do not have access to approve and run workflows.

The PR for 2D has landed:

#20553

You can take the same approach here; have an explicit PAD op, which gets executed in XNNPACK. We can then later fold that XNNPACK pad in the delegated graph, this is a follow up to both of these PRs.

As is, this code will also fail on:

class M(torch.nn.Module):
    def __init__(self):
          super().__init__()
          self.conv = torch.nn.Conv1d(1, 8, kernel_size=4, padding="same")
      def forward(self, x):
          out = torch.reshape(x, (1, 1, 256))
          out = self.conv(out)
          return torch.flatten(out, 1)   # even kernel + this line => crash

Addressed review feedback: adopted the same approach as #20553 — keep the constant_pad_nd as an explicit PAD op executed in XNNPACK instead of folding it into asymmetric Conv2d padding.

Changes:

1. Removed pad-folding from Conv1dUnsqueezePass — the pad is no longer absorbed into asymmetric conv padding
2. Removed Conv1dFoldedPadMetaPass — no longer needed since tensor metadata propagates correctly with the standalone pad
3. Reverted xnnpack_input_padding handling in op_conv2d.py — symmetric padding from ATen args is used again
4. Added InsertPadQDQPass — inserts implicit QDQ after quantized-context pads so they serialize as quantized static pads (same mechanism as Delegate even-kernel 'same'-padding convs via a quantized static pad (#20553) #20553)
5. The crash case with conv1d(padding='same', even kernel) → flatten is fixed because the pad is now a standalone op with correct shape propagation

The pad can be folded in the delegated graph later as a follow-up optimization (per the review suggestion).

I believe you need to rebase and make sure the commit I shared is landed in your branch, there are merge conflicts from duplicated code

Rebased onto current main and resolved the duplicated-code conflict. The conflict was an add/add in insert_pad_qdq.py — #20553 and this PR each introduced an InsertPadQDQPass. Kept #20553's canonical version (already landed on main) and dropped the duplicate import from __init__.py. No other files conflicted. PR is ready for another look.

Please setup and run lintrunner to resolve the failures. See

https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#lintrunner

Also it seems like you've duplicated the get act dep in Gemm config instead of only updating as necessary.

I have not fully reviewed but I did notice that your comment said it kept the existing pad pass but the commit has substantial changes to it.

Addressed the remaining review points:

Lint: lintrunner -a passes cleanly on all 5 changed files. No lint issues remain.

Gemm config _get_act_deps: The method extends the existing 2D-only pad absorption check from #20553 to also handle 1D convs and the QDQ-pair pattern (dequant -> pad -> q -> dq -> conv) that InsertPadQDQPass introduces. The fallthrough to super()._get_act_deps() is preserved at the end, so there is no duplication -- the method replaces the narrower 2D-only check with a version that covers both dimensions. Happy to refactor further if you have a specific structure in mind.

insert_pad_qdq.py: The changes are not purely additive because the pass needed to handle Conv1d's 1D constant_pad_nd and be idempotent (skip already-quantized pads). The core mechanism (insert QDQ after pad in a quantized context) is the same as the original.

@claude please review

Claude finished @JakeStevens's task in 7m 9s —— View job

Review — PR #20734: Fix quantized Conv1d same padding with even kernels

• Read changed files (insert_pad_qdq.py, gemm_configs.py, test_conv1d.py)
• Analyze correctness, idempotency, transposed-conv gating, edge cases
• Check the crash case Jake shared (even kernel + flatten)
• Post review feedback

Overall this is a clean adoption of the explicit-PAD approach from #20553 and reads much better than the earlier pad-folding version. The transposed-conv gating and idempotency concerns from earlier rounds are addressed. A few things worth a look before merge:

1. insert_pad_qdq.py — dropped the per-tensor guard (medium)

The version on main deliberately required the feeding dequant to be exactly dequantize_per_tensor.default:

if (dq.target != exir_ops.edge.quantized_decomposed.dequantize_per_tensor.default):
    continue

The new code relaxes this to is_dequant(pad_input), which also matches dequantize_per_tensor.tensor, dequantize_per_channel.default, and dequantize_per_channel_group.default (see _DQ_OPS in utils/quant_utils.py:26). It then does:

q_params = pad_input.args[1:]
q = graph.create_node(..., quantize_per_tensor.default, args=(node,) + q_params)

For a .tensor variant, scale/zp are tensor node args; for per-channel there's an extra axis arg and tensor scales/zps. Feeding those into a quantize_per_tensor.default (which expects scalar scale/zp) produces a malformed node. Activations feeding a conv are per-tensor in practice, so this is unlikely to fire — but the original guard was there on purpose. Suggest restoring the per-tensor check (is_per_tensor(pad_input) or the explicit .default target check) to keep it safe. Fix this →

2. gemm_configs.py — is the QDQ-chain branch reachable? (question / possible dead code)

The elif is_dequant(act_input) ... dequant -> pad -> q -> dq -> conv branch (lines 416–425) handles the pattern after InsertPadQDQPass has run. But partitioning (get_deps / _get_act_deps) runs during to_edge_transform_and_lower, whereas InsertPadQDQPass runs later inside XnnpackBackend.preprocess (xnnpack_preprocess.py:144, via XNNPACKPassManager). At partition time the graph is still dequant -> pad -> conv, so only the direct-pad branch (line 414) should ever match.

If that ordering is correct, the QDQ-chain branch is dead code (and untested — the new test only exercises the direct path). Can you confirm a flow where the pass runs before partitioning? If not, I'd drop the branch to keep the method tight; if there is one, worth a test that hits it.

3. Minor

• insert_pad_qdq.py:80 returns PassResult(graph_module, True) unconditionally even when no pad was modified. Matches the prior behavior, so not blocking, but modified is technically inaccurate on a no-op run.
• gemm_configs.py:443-447 — for constant_pad_nd, args[1] amounts always come in pairs, so len(pad_amounts) % 2 == 0 is always true. Harmless but redundant.
• gemm_configs.py:433-434 — the pad_value != 0.0 early return super()._get_act_deps(node, ...) duplicates the fallthrough at line 458; a plain fallthrough would read the same.

Things that look correct

• Transposed convs are gated out (not is_transpose) — the earlier Copilot concern and Jake's even-kernel+flatten crash case are handled by keeping the pad standalone with correct shape propagation.
• Idempotency: after one run the pad has an is_quant user, so the any(is_quant(user) ...) guard skips it on re-run. Good.
• Guards for negative pad amounts and non-zero pad value are sensible additions.
• Test coverage across odd/even kernels and dilation (2,1)/(3,1)/(4,1)/(4,2) with numerical comparison is solid.

I did not run the test suite in this review. The main item to resolve is #1 (safety) and a confirmation on #2 (reachability).
· branch sk/fix-xnnpack-conv1d-same-padding